Repository navigation
feat(v2): expose extract grounding as a top-level keyword - #168
Merged
Merged
Conversation
`options.grounding` skips the grounding stage, so every `extraction_metadata` leaf comes back with `ranges: null` and the request finishes faster. It is nested inside `options`, which extract does not surface, so it needed a hand-written top-level shorthand next to `strict` — a cross-SDK decision CONTRIBUTING.md reserves for a maintainer. The API owners asked for it in both SDKs, so `ade-typescript` gets the identical keyword with identical semantics. Both shorthands now fold into the SAME `options` object, so `_build_extract_body` collects them and attaches it once; assigning `body["options"]` per shorthand (what the code did while `strict` was alone) would drop whichever landed first. `bool()` rather than truthiness, since `grounding=False` is the only value the flag is ever passed for. Omitted or `None` leaves the key out and the server default applies. The live check asserts the returned `ranges` are all null rather than a 2xx: a shorthand that never reached the wire is not an error staging reports — it grounds anyway and comes back with real ranges. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The live contract test filters out leaves missing ranges, so it does not verify the stated every-leaf contract.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Exposes grounding= across all V2 extract entry points, folding it with strict into the nested API options.
Changes:
- Adds synchronous and asynchronous
groundingsupport. - Adds request-body and live contract coverage.
- Updates public documentation and cross-SDK alias guidance.
File summaries
| File | Description |
|---|---|
src/landingai_ade/resources/v2/extract.py |
Builds and sends the grounding option. |
src/landingai_ade/resources/v2/v2.py |
Exposes the top-level keyword. |
tests/api_resources/v2/test_extract.py |
Tests synchronous and job requests. |
tests/api_resources/v2/test_async_smoke.py |
Tests asynchronous entry points. |
tests/contract/test_v2_smoke.py |
Adds live grounding verification. |
README.md |
Documents user-facing behavior. |
api.md |
Updates API signatures and semantics. |
docs/v2-testing.md |
Documents implementation and testing details. |
CONTRIBUTING.md |
Records cross-SDK alias rules. |
.github/workflows/spec-sync.yml |
Updates spec-sync alias guidance. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…he ones that do Copilot review: filtering the live check to leaves that HAVE `ranges` let a leaf that dropped the key pass as if it had been checked. The schema is flat, so every top-level entry is a leaf and the documented contract is that all of them carry `ranges: null` with grounding off — assert the key's presence separately from its value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently handles omission and explicit false values across every sync and async extraction path with comprehensive coverage.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires
options.groundingas a top-levelgrounding=keyword onclient.v2.extractandclient.v2.extract_jobs.create, mirroringstrict. Requested by the API owners for both SDKs —ade-typescript#129is the identical change with identical semantics.grounding=Falseskips the grounding stage: everyextraction_metadataleaf comes back withranges: Noneand the request finishes faster. Preview; staging has it ahead of production.Notes:
optionsobject, so_build_extract_bodycollects them and attaches it once — assigningbody["options"]per shorthand (what the code did whilestrictwas alone) would drop whichever landed first.bool(), not truthiness:grounding=Falseis the only value the flag is ever passed for.Noneleaves the key out, so the server default applies (strictfalse,groundingtrue);optionsis omitted entirely when neither is given.rangesare allnullrather than a 2xx — a shorthand that never reached the wire is not an error staging reports, it just grounds anyway.Verified against staging:
grounding=False→ranges: null,grounding=True→ real ranges.